fix(gatsby): Fix argument order for onPreRouteUpdate
#30339
Conversation
The argument for `shouldComponentUpdate` is the *next* props, not the *previous* props, so `onPreRouteUpdate` was getting called with arguments in the reverse order.
gatsbot
bot
added
the
status: triage needed
StephenCleary
changed the title
StephenCleary
changed the title
|
This has been broken since 2020-10-08 with release gatsby@2.24.70. |
|
This makes sense to me in general. But can you provide an example project and steps to reproduce the issue? We need to verify both that the issue exists and that this PR fixes it. |
vladar
added
topic: reach/router and navigation
status: needs reproduction
|
Here ya go. There's one commit with the repro and steps are in the README. |
|
Could someone remove the "needs reproduction" tag? |
LekoArts
added
topic: frontend
|
@vladar I've just spent 2 hours debugging this: the arguments indeed come in the wrong order where location is actually the OLD location and prevLocation is the new location. The problem only exists for onPreRouteUpdate. The order is correct in onRouteUpdate. |
|
Hey! Thanks so much for opening this pull request! Sadly this PR got stale and didn't have any activity for some time. We're trying to do better with PR reviews! To get a better overview of all actionable PRs we're going through all open PRs and triage them. Since we won't be able to do everything and adding new features always means added maintenance burden, we have to be more picky about what's beneficial for the average user and the project itself longterm. Having said all this, we dropped the ball on this PR and we're sorry about not responding to it earlier. As said above, we want to do better in the future but here we should have communicated earlier what will happen with the PR. If you or someone else is still hitting this on Gatsby 4, please open a bug report at https://github.com/gatsbyjs/gatsby/issues/new?assignees=&labels=type%3A+bug&template=BUG_REPORT.yml and provide a minimal reproduction. Thanks! |
|
Also see #34298 |
LekoArts
removed
the
status: needs reproduction
LekoArts
changed the title
onPreRouteUpdate
This PR is completely untested; I wrote it in the GitHub UI.
Description
The argument for
shouldComponentUpdateis the next props, not the previous props, soonPreRouteUpdatewas getting called with arguments in the reverse order.Documentation
https://www.gatsbyjs.com/docs/reference/config-files/gatsby-browser/#onPreRouteUpdate
No doc update necessary; this PR brings the behavior in line with the existing docs.
Related Issues
Fixes #34298